Remove optimistic Concierge thinking indicator#88747
Remove optimistic Concierge thinking indicator#88747chiragsalian wants to merge 6 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Removes the client-side “Concierge is thinking…” optimistic indicator so the UI shows AgentZero processing state only when the backend provides a processing label.
Changes:
- Removed the
kickoffWaitingIndicatoroptimistic flow fromAgentZeroStatusContext(including localization dependency and timeout logic). - Stopped triggering the optimistic indicator on message send (
useComposerSubmit). - Updated unit tests to reflect server-driven-only processing state.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/unit/AgentZeroStatusContextTest.ts | Removes tests/mocks for the optimistic indicator and updates expectations to server-label-only behavior. |
| src/pages/inbox/report/ReportActionCompose/useComposerSubmit.ts | Stops calling the removed kickoffWaitingIndicator action on submit. |
| src/pages/inbox/AgentZeroStatusContext.tsx | Deletes optimistic state/actions and derives processing/label state solely from the server-provided indicator. |
Comments suppressed due to low confidence (1)
src/pages/inbox/AgentZeroStatusContext.tsx:103
- This component updates state during render (
setPrevServerLabel/setReasoningHistoryinside anifat the top level). Even though the condition prevents an infinite loop, setting state during render is an anti-pattern in React and can produce warnings/unpredictable behavior with concurrent rendering. Move this logic into auseEffectthat runs whenserverLabelchanges (and clearreasoningHistorythere when transitioning truthy → falsy).
// Clear reasoning when processing ends (server label transitions from truthy → falsy)
const [prevServerLabel, setPrevServerLabel] = useState(serverLabel);
if (prevServerLabel !== serverLabel) {
setPrevServerLabel(serverLabel);
if (prevServerLabel && !serverLabel && reasoningHistory.length > 0) {
setReasoningHistory([]);
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@ahmedGaber93 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
Ready for review. You won't be able to test the full PR since it requires web-e and auth to be deployed test it fully. But you can test that we've removing optimistic setting of "Concierge is thinking.." |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
It has been removed and works as expected. I’ll run a final test after the BE PR is deployed. |
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
setOptimisticStartTime was deleted along with the optimistic indicator, but the kickoff useEffect that called it remained, breaking typecheck. The kickoff path's only purpose was starting the optimistic timer, so the entire chain (Onyx key, setter/clearer in Report, set call from useAskConcierge) is now dead and removed.
|
BE code is live. Code should be ready for review. |
HOLD till,
are live.
Explanation of Change
Removes the client-side optimistic "Concierge is thinking..." indicator entirely. Backend is now the sole source of truth for whether to show the indicator.
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/621231
PROPOSAL:
Tests
Setup,
Manual verification (combined with the backend PRs):
Concierge DM
Another test message to test routing, please respond with "test me two".Admins room chat
Another test message to test routing, please respond with "test me two".Offline tests
The optimistic indicator was already gated on
!isOffline; offline behavior is unchanged — messages queue normally, no indicator renders until online and server sends a label.QA Steps
Setup,
Same as the manual verification above.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari